Repository navigation
Conversation
Atharva-Kanherkar
force-pushed
the
fix/responses-output-hosted-tool-items
branch
from
August 8, 2026 13:29
d1212f2 to
45c065b
Compare
Contributor
Author
|
hey @bxyu-nvidia, sorry for the direct ping. same bug as #1910, just the remaining output-call types (web search, file search, computer use, image gen, code interpreter, local shell, custom). they were missing from the union, so the proxy 500s after the provider already succeeded. filed as #2436. tagging you since you wrote most of @cmunley1 it also needs vetting before ci can run. |
ananthsub
approved these changes
Aug 10, 2026
The OpenAI Responses API emits additional output item variants for hosted tools and client-executed custom tools. None of these variants were in NeMoGymResponseInputItem, and NeMoGymResponseOutputItem is an alias of it, so NeMoGymResponse validation rejected valid upstream payloads and the openai_model proxy turned a successful response into HTTP 500. Add NeMoGym wrappers for the seven missing variants, following the existing hosted-MCP item pattern and inheriting the pinned SDK models. The SDK includes these items in both ResponseOutputItem and ResponseInputItemParam, so extending the shared union also permits later turns to echo prior output items as input. Verified against api.openai.com with a forced web-search request: the model server returned 500 before this change and 200 after it. Fixes NVIDIA-NeMo#2436 Signed-off-by: Atharva-Kanherkar <142440039+Atharva-Kanherkar@users.noreply.github.com>
Checkpoint: define discriminator, transcript-boundary, canonicalization, and metadata expectations before applying review fixes. Signed-off-by: Atharva-Kanherkar <142440039+Atharva-Kanherkar@users.noreply.github.com>
Checkpoint: preserve upstream required type fields and reject untagged output items before union coercion. Reviewed against testing/fix-responses-output-hosted-tool-items.md. Signed-off-by: Atharva-Kanherkar <142440039+Atharva-Kanherkar@users.noreply.github.com>
Checkpoint: classify every model-output call type, keep input-only transcripts on the input side, and retain tool-only model output. Reviewed against testing/fix-responses-output-hosted-tool-items.md. Signed-off-by: Atharva-Kanherkar <142440039+Atharva-Kanherkar@users.noreply.github.com>
Signed-off-by: Atharva-Kanherkar <142440039+Atharva-Kanherkar@users.noreply.github.com>
Signed-off-by: Atharva-Kanherkar <142440039+Atharva-Kanherkar@users.noreply.github.com>
ananthsub
force-pushed
the
fix/responses-output-hosted-tool-items
branch
from
August 10, 2026 16:02
60e03df to
465cdd7
Compare
Contributor
Author
|
thanks!<3 |
ananthsub
added a commit
that referenced
this pull request
Aug 10, 2026
…sponse (#2461) `NeMoGymResponseOutputItem` gained a `BeforeValidator` in #2438 that rejects dicts without a `type` field. `test_run_workflow` mocks the /verify echo with an untagged item, `{"role": "user", "content": "obs"}`, so validating that payload into `AviaryAgentVerifyResponse` now raises. Without the guard, an untagged output-call dict validates as the wrong union member: a web-search call with no `type` becomes `NeMoGymResponseMcpListTools`. The Aviary integration is unaffected. `AviaryAgent.run` posts `verify_request.model_dump()`, and `AviaryResourcesServer.verify` echoes that back, so each item carries its `type`. Signed-off-by: Ananth Subramaniam <ansubramania@nvidia.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #2436
Problem
The Responses API can emit output-call items that were missing from
NeMoGymResponseInputItem. BecauseNeMoGymResponseOutputItemshares that union, valid upstream responses containing those items failed local validation and the model proxy returned HTTP 500 after the provider had succeeded.Changes
typediscriminator.Web search, file search, image generation, and code interpreter are provider-executed. Computer actions, local-shell commands, and custom calls are returned for the integrator to execute.
Behavior change: the no-match case in
split_responses_input_output_itemsSeparately from the new item types, this changes what happens when no output boundary is found at all. Calling it out explicitly since it is not implied by the title.
The old loop read the leaked loop variable after
break, so when nothing matched it fell through withi == len(items) - 1and the final item was always classified as output. The new code initializessplit_at = len(items), so a transcript with no output boundary is now treated as entirely input.[function_call_output]([], [function_call_output])([function_call_output], [])[user_message, function_call_output]([user_message], [function_call_output])([user_message, function_call_output], [])[user_message, function_call]([user_message], [function_call])The old result was an off-by-one: a trailing user or tool item was reported as model output even though the transcript contained no model turn. Transcripts that do contain a boundary item are unaffected, because the loop still breaks at the same index.
Worth noting that a single-item transcript of one of the newly recognized types, for example
[web_search_call], returns the same pair before and after, because the old fallback happened to land on the correct split for that case. The two changes mask each other there, which is why the existing tests did not surface the difference.Effect on the two callers
In both callers
output_itemsbecomesresponse.output, so the only transcripts affected are those with no output boundary anywhere.responses_api_agents/tau2/app.py: the first split breaks at the leading assistant message, per the comment there, so only the second split over the remainder is in scope. If that remainder is entirely user and tool items, its trailing item is no longer reported as model output. An emptyoutput_itemsslices safely, sooutput_items[:1]andoutput_items[1:]still behave.responses_api_agents/swe_agents/app.py: only the non-replay branch calls this, since the replay branch computes its ownsplit_at. A trajectory with no model turn now yields an emptyoutputrather than one mislabeled trailing item.In both cases the new result is the accurate one, and neither path raises on an empty list.
Verification
1580 passed, 369 deselected, 5 subtests passedin the complete non-sandbox unit suite.117 passedacross the response schemas, converter, model proxy, and conversational simulation suites.git diff --checkpassed.web_search_calloutput forgpt-4.1-mini,gpt-5.4-mini,gpt-5.6-luna, andgpt-5.6-terra; the proxy failure was reproduced before the schema fix and resolved afterward.